feat(d_snd_harp_song_mrg): isPlayingHarpRelated and ancestors#264
feat(d_snd_harp_song_mrg): isPlayingHarpRelated and ancestors#264elijah-thomas774 merged 5 commits intozeldaret:mainfrom
Conversation
| u8 dSndHarpSongMgr_c::fun_80381150() { | ||
| if (this->isContinuousStrumming()) { | ||
| return this->field_0x042; |
There was a problem hiding this comment.
| u8 dSndHarpSongMgr_c::fun_80381150() { | |
| if (this->isContinuousStrumming()) { | |
| return this->field_0x042; | |
| bool dSndHarpSongMgr_c::fun_80381150() { | |
| if (isContinuousStrumming()) { | |
| return field_0x042; |
There was a problem hiding this comment.
By making field_0x42 a bool things can work out better.
/* 0x042 */ bool field_0x042;
There was a problem hiding this comment.
The change works much better now. I was initially hesitant because I didn’t want to introduce side‑effects elsewhere in the codebase. Since the field was previously declared as a u8, I assumed that was the intended type.
There was a problem hiding this comment.
Sure u8 and bool conversions are pretty common because initially they appear the same until we have situations such as this.
Similar with the u16 vs s16 vs mAng, there may be cases were changing the type is needed just for one area.
I’ll enable the pr checker run so you can see the effects, but there is a way to setup ninja to get a baseline, then a command to check for and regressions. I think steps are somewhere on discord
Report for SOUE01 (67eda44 - 26f4a90)📈 Matched code: 20.95% (+0.00%, +124 bytes) ✅ 3 new matches
|
|
Note: for changes to actually show up towards completion percentage, you may need to map the symbol from objdiff to the respective symbols.txt. For example: - isPlayingHarpRelated = .text:0x80381120; // type:function size:0x28
+ isPlayingHarpRelated__17dSndHarpSongMgr_cFv = .text:0x80381120; // type:function size:0x28This can be grabbed from right clicking the function in objdiff: Also: The order in which the function defined matters, so try to keep them in the right order |
|
I mapped the symbols and now it shows 100 %, so I was missing that step. I've also renamed and reordered the function to match the original names/order. |
Updated field_0x042 from u8 to bool to better reflect its binary nature. Adjusted related logic accordingly.
- Rename fn_80381150 to match real name - Reordered functions
Asiern
force-pushed
the
d_snd_harp_song_mgr
branch
from
a8f637f to
26f4a90
Compare
2 participants
Added isPlayingHarpRelated and its ancestor functions. Used by d_t_harp.
isPlayingHarpRelated– 100 %fun_80381150– 100%isContinuousStrumming()– 100 %